Skip to content

Conversation

@mt-dfrey
Copy link
Contributor

@mt-dfrey mt-dfrey commented Oct 5, 2023

  • doc(sdk) add TSDoc comments to public API
  • chore(docs) run yarn build:docs
  • doc(sdk) replace markdown API doc with links typedoc
  • doc(sdk) update readme with badge maintainer instructions

baac479 is a large diff because it generates a lot of static pages. You can ignore it for review (as long as you review the previous commit that makes changes to the typescript source files 😉 )

@mt-dfrey mt-dfrey requested a review from a team as a code owner October 5, 2023 08:24
@mt-dfrey mt-dfrey force-pushed the update-documentation branch from a692ddd to 2a3ec73 Compare October 5, 2023 08:38
@mt-dfrey mt-dfrey force-pushed the update-documentation branch from 2a3ec73 to 7fd3f27 Compare October 5, 2023 08:53
@mt-dfrey mt-dfrey requested a review from mt-rorykelly October 6, 2023 01:12
src/typings.ts Outdated
token_type: string;
/** created at in seconds */
created_at: number;
/** expiry in seconds */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that seconds after created_at? If so, maybe we can be explicit about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the description from the RFC

The lifetime in seconds of the access token.
For example, the value "3600" denotes that the access token will expire in one hour from the time the response was generated.

and added a link to the spec on the type.

src/index.ts Outdated
* The only supported flow is authorization code grant with PKCE [PKCE](https://auth0.com/docs/flows/concepts/auth-code-pkce)
* If the user is not logged in yet this will show the login screen and redirect the guest to the consent screen after they log in.
* After the guest consents, they will be redirected to the redirect URI with an authorization code.
* If the user is already logged and has granted consent in the redirection happens immediately.
Copy link

@mt-ronkorving mt-ronkorving Oct 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 108: "user" + "guest"
Line 109: "guest"
Line 110: "user"

I don't know which word is canonical, but I think we should stick to one.

Another suggestion on this line: "logged" -> "logged in"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced all guest with user in the docs.
Some of our APIs still have guest (like the scope guest_read and some API response objects).
But this is 99% consistent now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and yes, logged in is better 😁

@mt-dfrey mt-dfrey mentioned this pull request Oct 6, 2023
mt-dfrey and others added 2 commits October 6, 2023 11:28
Co-authored-by: Rory Kelly <90672264+mt-rorykelly@users.noreply.github.com>
@mt-dfrey mt-dfrey force-pushed the update-documentation branch from 2e36d2f to a3660a8 Compare October 6, 2023 02:29
@mt-dfrey
Copy link
Contributor Author

mt-dfrey commented Oct 6, 2023

@mt-ronkorving @mt-rorykelly
Thanks for your your feedback, I addressed all points.

@mt-dfrey mt-dfrey merged commit 3099aee into master Oct 6, 2023
@mt-dfrey mt-dfrey deleted the update-documentation branch October 6, 2023 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants